Fix: scalar_data_test check tensor reads uninitialized device slot - #1449
Conversation
The `check` output tensor was sized 10, but the orchestration writes only check[0..8] via set_tensor_data (9 values). Output-tensor slots are not seeded from the host buffer on device, so the never-written check[9] read whatever the device output buffer held: 0.0 when the test runs in isolation (matching the zero golden), but recycled buffer residue (~9.4e-4) under the full onboard suite that shares one device — an intermittent golden mismatch on st-onboard-a2a3. Size `check` to exactly the 9 written slots. Verified onboard on a2a3: a size-9 sentinel-seeded run passes (all slots overwritten), and the failing slot-9 residue is gone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe scalar data test’s ChangesScalar data test
Estimated code review effort: 1 (Trivial) | ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fixes hw-native-sys#1483 `tests/st/a2a3/host_build_graph/bgemm` declared its `C` argument `D.OUT`, but the task graph read-modify-writes it: `p_add.add_inout(c_view)` feeds `kernel_tile_add`, whose first `k` iteration `TLOAD`s `C` before any task has written it. The runtime stages only IN/INOUT tensors host->device (`runtime_maker.cpp`, "pure write-only OUTPUT buffers are never read by the kernel"), and since hw-native-sys#1365 it no longer zero-fills OUT buffers either. So `C` accumulated onto whatever the device allocation happened to hold. On sim that allocation is a plain `malloc` block on a process-lifetime worker: fresh zeros when the case runs alone, a previous case's bytes in a full-suite run — hence a golden mismatch of 1e25-1e36 that reproduced only under the full suite. Same class as hw-native-sys#1449. Declare `C` INOUT, matching the two sibling bgemm variants (`examples/a5/tensormap_and_ringbuffer/bgemm`, `examples/a2a3/tensormap_and_ringbuffer/benchmark_bgemm`), which already tag their accumulator that way. Seed `C` with a non-zero base (0.25) so the staging is load-bearing and the case is deterministic: golden is now `C_init + A @ B`, which a zeroed or unstaged device buffer cannot match. Without the signature fix this fails every run instead of one in two or three — verified, it reproduces the reported 1e35 magnitude on the first attempt. Drop the `pytest.mark.skip` that quarantined the case: the underlying defect is fixed, so the case runs in CI again. Verified: bgemm passes 3/3 in isolation on a2a3sim and onboard on a2a3; the full `pytest examples tests/st --platform a2a3sim` suite is green 3/3 runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
) Fixes #1483 `tests/st/a2a3/host_build_graph/bgemm` declared its `C` argument `D.OUT`, but the task graph read-modify-writes it: `p_add.add_inout(c_view)` feeds `kernel_tile_add`, whose first `k` iteration `TLOAD`s `C` before any task has written it. The runtime stages only IN/INOUT tensors host->device (`runtime_maker.cpp`, "pure write-only OUTPUT buffers are never read by the kernel"), and since #1365 it no longer zero-fills OUT buffers either. So `C` accumulated onto whatever the device allocation happened to hold. On sim that allocation is a plain `malloc` block on a process-lifetime worker: fresh zeros when the case runs alone, a previous case's bytes in a full-suite run — hence a golden mismatch of 1e25-1e36 that reproduced only under the full suite. Same class as #1449. Declare `C` INOUT, matching the two sibling bgemm variants (`examples/a5/tensormap_and_ringbuffer/bgemm`, `examples/a2a3/tensormap_and_ringbuffer/benchmark_bgemm`), which already tag their accumulator that way. Seed `C` with a non-zero base (0.25) so the staging is load-bearing and the case is deterministic: golden is now `C_init + A @ B`, which a zeroed or unstaged device buffer cannot match. Without the signature fix this fails every run instead of one in two or three — verified, it reproduces the reported 1e35 magnitude on the first attempt. Drop the `pytest.mark.skip` that quarantined the case: the underlying defect is fixed, so the case runs in CI again. Verified: bgemm passes 3/3 in isolation on a2a3sim and onboard on a2a3; the full `pytest examples tests/st --platform a2a3sim` suite is green 3/3 runs.
Summary
scalar_data_testintermittently failsst-onboard-a2a3with a golden mismatch oncheck(max_diff≈9.4e-4,rtol/atol=1e-5). Root-caused on a2a3 silicon.Root cause
The
checkoutput tensor is sized 10, but the orchestration writes onlycheck[0..8](9 values viaset_tensor_data). Output-tensor slots are not seeded from the host buffer on device (confirmed on hardware: a 123.0 host sentinel reads back as0.0), so the never-writtencheck[9]reads whatever the device output buffer holds:0.0→ matches the zero golden → passes.~9.4e-4) → golden mismatch. This is why it's intermittent and survives re-runs.Decisive evidence (a2a3, sentinel-seeded
check):Fix
Size
checkto exactly the 9 written slots. Verified onboard on a2a3: a size-9 sentinel-seeded run passes (every slot overwritten). No golden change (it only sets 0..8); sim is bit-exact and unaffected. Unrelated to #1436 (this is alevel=2device-runtime test).🤖 Generated with Claude Code